checkout/commit: Use glnx_regfile_copy_bytes() if possible

Rather than `g_output_stream_splice()`, where the input is a regular
file.

See https://github.com/GNOME/libglnx/pull/44 for some more information.

I didn't try to measure the performance difference, but seeing the
read()/write() to/from userspace mixed in with the pointless `poll()` annoyed me
when reading strace.

As a bonus, we will again start using reflinks (if available) for `/etc`,
which is a regression from the https://github.com/ostreedev/ostree/pull/797
changes (which before used `glnx_file_copy_at()`).

Also, for the first time we'll use reflinks when doing commits from file-backed
content. This happens in `rpm-ostree compose tree` today for example.

Update submodule: libglnx

Closes: #817
Approved by: jlebon

diff --git a/libglnx b/libglnx
index 602fdd93cb7a339c6b5749eee73df926429a5ab8..3a4d0f4684f4653338c4756c8a1abfc6b5738467 160000 (submodule)
--- a/libglnx
+++ b/libglnx
@@ -1 +1 @@
-Subproject commit 602fdd93cb7a339c6b5749eee73df926429a5ab8
+Subproject commit 3a4d0f4684f4653338c4756c8a1abfc6b5738467

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 36be420b6ee69750989ece0f47c7f2e648a254df..50967769ca3972ee8c364bca6b3cf1f24ee2b472 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -102,7 +102,7 @@ fsync_is_enabled (OstreeRepo   *self,
 static gboolean
 write_regular_file_content (OstreeRepo            *self,
                             OstreeRepoCheckoutAtOptions *options,
-                            GOutputStream         *output,
+                            int                    outfd,
                             GFileInfo             *file_info,
                             GVariant              *xattrs,
                             GInputStream          *input,
@@ -110,40 +110,54 @@ write_regular_file_content (OstreeRepo            *self,
                             GError               **error)
 {
   const OstreeRepoCheckoutMode mode = options->mode;
+  g_autoptr(GOutputStream) outstream = NULL;
 
-  if (g_output_stream_splice (output, input, 0,
-                              cancellable, error) < 0)
-    return FALSE;
+  if (G_IS_FILE_DESCRIPTOR_BASED (input))
+    {
+      int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input);
+      guint64 len = g_file_info_get_size (file_info);
 
-  if (!g_output_stream_flush (output, cancellable, error))
-    return FALSE;
+      if (glnx_regfile_copy_bytes (infd, outfd, (off_t)len, TRUE) < 0)
+        return glnx_throw_errno_prefix (error, "regfile copy");
+    }
+  else
+    {
+      outstream = g_unix_output_stream_new (outfd, FALSE);
+      if (g_output_stream_splice (outstream, input, 0,
+                                  cancellable, error) < 0)
+        return FALSE;
 
-  int fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)output);
+      if (!g_output_stream_flush (outstream, cancellable, error))
+        return FALSE;
+    }
 
   if (mode != OSTREE_REPO_CHECKOUT_MODE_USER)
     {
-      if (TEMP_FAILURE_RETRY (fchown (fd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"),
+      if (TEMP_FAILURE_RETRY (fchown (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"),
                                       g_file_info_get_attribute_uint32 (file_info, "unix::gid"))) < 0)
         return glnx_throw_errno (error);
 
-      if (TEMP_FAILURE_RETRY (fchmod (fd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0)
+      if (TEMP_FAILURE_RETRY (fchmod (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0)
         return glnx_throw_errno (error);
 
       if (xattrs)
         {
-          if (!glnx_fd_set_all_xattrs (fd, xattrs, cancellable, error))
+          if (!glnx_fd_set_all_xattrs (outfd, xattrs, cancellable, error))
             return FALSE;
         }
     }
 
   if (fsync_is_enabled (self, options))
     {
-      if (fsync (fd) == -1)
+      if (fsync (outfd) == -1)
         return glnx_throw_errno (error);
     }
 
-  if (!g_output_stream_close (output, cancellable, error))
-    return FALSE;
+  if (outstream)
+    {
+      if (!g_output_stream_close (outstream, cancellable, error))
+        return FALSE;
+    }
 
   return TRUE;
 }
@@ -231,7 +245,6 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
   else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
     {
       glnx_fd_close int temp_fd = -1;
-      g_autoptr(GOutputStream) temp_out = NULL;
       guint32 file_mode;
       GLnxLinkTmpfileReplaceMode replace_mode;
 
@@ -244,7 +257,6 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
                                           &temp_fd, &temp_filename,
                                           error))
         return FALSE;
-      temp_out = g_unix_output_stream_new (temp_fd, FALSE);
 
       if (sepolicy_enabled)
         {
@@ -258,7 +270,7 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
             return glnx_throw_errno_prefix (error, "Setting security.selinux");
         }
 
-      if (!write_regular_file_content (repo, options, temp_out, file_info, xattrs, input,
+      if (!write_regular_file_content (repo, options, temp_fd, file_info, xattrs, input,
                                        cancellable, error))
         return FALSE;
 

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 664016b809a8f8a576f2d6a21d902842f5530714..2e9f92439d275a3923dea83b36f2c9dd29784e0d 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -458,29 +458,19 @@ add_size_index_to_metadata (OstreeRepo        *self,
 }
 
 static gboolean
-fallocate_stream (GFileDescriptorBased      *stream,
-                  goffset                    size,
-                  GCancellable              *cancellable,
-                  GError                   **error)
+ot_fallocate (int fd, goffset size, GError **error)
 {
-  gboolean ret = FALSE;
-  int fd = g_file_descriptor_based_get_fd (stream);
+  if (size == 0)
+    return TRUE;
 
-  if (size > 0)
+  int r = posix_fallocate (fd, 0, size);
+  if (r != 0)
     {
-      int r = posix_fallocate (fd, 0, size);
-      if (r != 0)
-        {
-          /* posix_fallocate is a weird deviation from errno standards */
-          errno = r;
-          glnx_set_error_from_errno (error);
-          goto out;
-        }
+      /* posix_fallocate is a weird deviation from errno standards */
+      errno = r;
+      return glnx_throw_errno_prefix (error, "fallocate");
     }
-
-  ret = TRUE;
- out:
-  return ret;
+  return TRUE;
 }
 
 gboolean
@@ -510,11 +500,10 @@ _ostree_repo_open_content_bare (OstreeRepo          *self,
                                           &fd, &temp_filename, error))
         goto out;
 
-      ret_stream = g_unix_output_stream_new (fd, TRUE);
-      
-      if (!fallocate_stream ((GFileDescriptorBased*)ret_stream, content_len,
-                             cancellable, error))
+      if (!ot_fallocate (fd, content_len, error))
         goto out;
+
+      ret_stream = g_unix_output_stream_new (fd, TRUE);
     }
 
   ret = TRUE;
@@ -569,7 +558,6 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
                                               GCancellable *cancellable,
                                               GError **error)
 {
-  g_autoptr(GOutputStream) temp_out = NULL;
   glnx_fd_close int temp_fd = -1;
   g_autofree char *temp_filename = NULL;
 
@@ -577,20 +565,27 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
                                       &temp_fd, &temp_filename,
                                       error))
     return FALSE;
-  temp_out = g_unix_output_stream_new (temp_fd, FALSE);
 
-  if (!fallocate_stream ((GFileDescriptorBased*)temp_out, length,
-                         cancellable, error))
+  if (!ot_fallocate (temp_fd, length, error))
     return FALSE;
 
-  if (g_output_stream_splice (temp_out, input, 0,
-                              cancellable, error) < 0)
-    return FALSE;
-  if (fchmod (temp_fd, 0644) < 0)
+  if (G_IS_FILE_DESCRIPTOR_BASED (input))
     {
-      glnx_set_error_from_errno (error);
-      return FALSE;
+      int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input);
+      if (glnx_regfile_copy_bytes (infd, temp_fd, (off_t)length, TRUE) < 0)
+        return glnx_throw_errno_prefix (error, "regfile copy");
     }
+  else
+    {
+      g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (temp_fd, FALSE);
+      if (g_output_stream_splice (temp_out, input, 0,
+                                  cancellable, error) < 0)
+        return FALSE;
+    }
+
+  if (fchmod (temp_fd, 0644) < 0)
+    return glnx_throw_errno_prefix (error, "fchmod");
+
   *out_fd = temp_fd; temp_fd = -1;
   *out_path = g_steal_pointer (&temp_filename);
   return TRUE;

diff --git a/tests/ci-commitmessage-submodules.sh b/tests/ci-commitmessage-submodules.sh
index 77a4e1a16c845f7f1b651bca75c544dcbc83555b..2403579d6a64982400e27c7b41116d60451d898c 100755 (executable)
--- a/tests/ci-commitmessage-submodules.sh
+++ b/tests/ci-commitmessage-submodules.sh
@@ -40,6 +40,7 @@ git log --pretty=oneline origin/master.. | while read logline; do
             echo "Commit $commit modifies submodule: $submodule"
             expected_match="Update submodule: $submodule"
             if ! grep -q -e "$expected_match" ${tmpd}/log.txt; then
+                sed -e 's,^,# ,' < ${tmpd}/log.txt
                 echo "error: Commit message for ${commit} changes a submodule, but does not match regex ${expected_match}"
                 exit 1
             fi